chore(nextjs): Improve machine auth verification calls #6367
Conversation
🦋 Changeset detectedLatest commit: cc4358b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
📝 WalkthroughWalkthroughThe changes update the Next.js package's server-side authentication flow by introducing a new Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (7)**/*.{js,jsx,ts,tsx}📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Files:
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Files:
packages/**/*.{ts,tsx}📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Files:
packages/**/*.{ts,tsx,d.ts}📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Files:
**/*.{ts,tsx}📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Files:
**/*.{js,ts,tsx,jsx}📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Files:
**/*⚙️ CodeRabbit Configuration File
Files:
🧬 Code Graph Analysis (1)packages/nextjs/src/server/data/getAuthDataFromRequest.ts (8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
🔇 Additional comments (5)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/nextjs/src/app-router/server/auth.ts(1 hunks)packages/nextjs/src/server/__tests__/getAuthDataFromRequest.test.ts(1 hunks)packages/nextjs/src/server/clerkMiddleware.ts(2 hunks)packages/nextjs/src/server/data/getAuthDataFromRequest.ts(3 hunks)packages/nextjs/src/server/utils.ts(7 hunks)
📓 Path-based instructions (9)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/nextjs/src/server/__tests__/getAuthDataFromRequest.test.tspackages/nextjs/src/app-router/server/auth.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/nextjs/src/server/__tests__/getAuthDataFromRequest.test.tspackages/nextjs/src/app-router/server/auth.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/nextjs/src/server/__tests__/getAuthDataFromRequest.test.tspackages/nextjs/src/app-router/server/auth.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/nextjs/src/server/__tests__/getAuthDataFromRequest.test.tspackages/nextjs/src/app-router/server/auth.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/nextjs/src/server/__tests__/getAuthDataFromRequest.test.tspackages/nextjs/src/app-router/server/auth.ts
packages/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Unit tests should use Jest or Vitest as the test runner.
Files:
packages/nextjs/src/server/__tests__/getAuthDataFromRequest.test.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/nextjs/src/server/__tests__/getAuthDataFromRequest.test.tspackages/nextjs/src/app-router/server/auth.ts
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/typescript.mdc)
**/__tests__/**/*.{ts,tsx}: Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces
Files:
packages/nextjs/src/server/__tests__/getAuthDataFromRequest.test.ts
**/*
⚙️ CodeRabbit Configuration File
**/*: If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.Whenever reviewing a pull request, if there are any changes that could impact security, always tag
clerk/securityin the PR.Security-impacting changes include, but are not limited to:
- Changes to authentication logic or mechanisms (e.g. login, session handling, token issuance)
- Any modification to access control, authorization checks, or role-based permissions
- Introduction or modification of hashing algorithms, signature verification, or cryptographic primitives
- Handling of sensitive data (e.g. passwords, tokens, secrets, PII)
- Integration with external identity providers (e.g. SSO, OAuth, OpenID Connect)
- Modifications to security headers, cookie flags, CORS policies, or CSRF protections
- Bypass mechanisms (e.g. feature flags, testing overrides) that could weaken protections
- Changes to rate limiting, abuse prevention, or input validation
If you're unsure whether a change is security-relevant, err on the side of caution and tag
clerk/security.Any time that you tag
clerk/security, please do so explicitly in a code comment, rather than within a collapsed section in a coderabbit comment, such as the "recent review details" section. If you do use the team name in any thinking or non-direct-code-comment content, it can be referred to as "clerk security team" to avoid accidentally printing the tag which sends a notification to the team.
Files:
packages/nextjs/src/server/__tests__/getAuthDataFromRequest.test.tspackages/nextjs/src/app-router/server/auth.ts
🧬 Code Graph Analysis (1)
packages/nextjs/src/app-router/server/auth.ts (1)
packages/backend/src/tokens/tokenTypes.ts (2)
TokenType(1-6)TokenType(11-11)
🪛 Biome (1.9.4)
packages/nextjs/src/server/__tests__/getAuthDataFromRequest.test.ts
[error] 47-47: Don't focus the test.
The 'only' method is often used for debugging or during implementation. It should be removed before deploying to production.
Consider removing 'only' to ensure all tests are executed.
Unsafe fix: Remove focus from test.
(lint/suspicious/noFocusedTests)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/nextjs/src/server/clerkMiddleware.ts
- packages/nextjs/src/server/utils.ts
- packages/nextjs/src/server/data/getAuthDataFromRequest.ts
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/nextjs/src/server/__tests__/getAuthDataFromRequest.test.tspackages/nextjs/src/app-router/server/auth.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/nextjs/src/server/__tests__/getAuthDataFromRequest.test.tspackages/nextjs/src/app-router/server/auth.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/nextjs/src/server/__tests__/getAuthDataFromRequest.test.tspackages/nextjs/src/app-router/server/auth.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/nextjs/src/server/__tests__/getAuthDataFromRequest.test.tspackages/nextjs/src/app-router/server/auth.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/nextjs/src/server/__tests__/getAuthDataFromRequest.test.tspackages/nextjs/src/app-router/server/auth.ts
packages/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Unit tests should use Jest or Vitest as the test runner.
Files:
packages/nextjs/src/server/__tests__/getAuthDataFromRequest.test.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/nextjs/src/server/__tests__/getAuthDataFromRequest.test.tspackages/nextjs/src/app-router/server/auth.ts
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/typescript.mdc)
**/__tests__/**/*.{ts,tsx}: Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces
Files:
packages/nextjs/src/server/__tests__/getAuthDataFromRequest.test.ts
**/*
⚙️ CodeRabbit Configuration File
**/*: If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.Whenever reviewing a pull request, if there are any changes that could impact security, always tag
clerk/securityin the PR.Security-impacting changes include, but are not limited to:
- Changes to authentication logic or mechanisms (e.g. login, session handling, token issuance)
- Any modification to access control, authorization checks, or role-based permissions
- Introduction or modification of hashing algorithms, signature verification, or cryptographic primitives
- Handling of sensitive data (e.g. passwords, tokens, secrets, PII)
- Integration with external identity providers (e.g. SSO, OAuth, OpenID Connect)
- Modifications to security headers, cookie flags, CORS policies, or CSRF protections
- Bypass mechanisms (e.g. feature flags, testing overrides) that could weaken protections
- Changes to rate limiting, abuse prevention, or input validation
If you're unsure whether a change is security-relevant, err on the side of caution and tag
clerk/security.Any time that you tag
clerk/security, please do so explicitly in a code comment, rather than within a collapsed section in a coderabbit comment, such as the "recent review details" section. If you do use the team name in any thinking or non-direct-code-comment content, it can be referred to as "clerk security team" to avoid accidentally printing the tag which sends a notification to the team.
Files:
packages/nextjs/src/server/__tests__/getAuthDataFromRequest.test.tspackages/nextjs/src/app-router/server/auth.ts
🧬 Code Graph Analysis (1)
packages/nextjs/src/app-router/server/auth.ts (1)
packages/backend/src/tokens/tokenTypes.ts (2)
TokenType(1-6)TokenType(11-11)
🪛 Biome (1.9.4)
packages/nextjs/src/server/__tests__/getAuthDataFromRequest.test.ts
[error] 47-47: Don't focus the test.
The 'only' method is often used for debugging or during implementation. It should be removed before deploying to production.
Consider removing 'only' to ensure all tests are executed.
Unsafe fix: Remove focus from test.
(lint/suspicious/noFocusedTests)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
| if (authObject.tokenType === TokenType.SessionToken) { | ||
| return Object.assign(authObject, { redirectToSignIn, redirectToSignUp }); | ||
| } | ||
|
|
||
| return authObject; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Conditional redirect methods based on token type - approve with security review needed.
This change correctly restricts redirect methods to session tokens only, which makes semantic sense since machine tokens, API keys, and OAuth tokens are for programmatic access and don't need user redirect capabilities.
However, this modifies core authentication logic and should be reviewed by the security team.
Please tag clerk/security for review of this authentication logic change.
Also, ensure test coverage exists for both branches of this conditional logic:
If test coverage is missing, please add tests to verify:
- Session tokens return authObject with redirect methods
- Non-session tokens return authObject without redirect methods
- The redirect methods work correctly when present
🏁 Script executed:
#!/bin/bash
# Description: Check for test coverage of conditional redirect method behavior
# Search for tests related to auth function with different token types
rg -A 10 -B 5 "auth.*tokenType|tokenType.*auth" --type ts --type js
# Search for tests of redirect methods on auth objects
rg -A 10 -B 5 "redirectToSignIn|redirectToSignUp" --type ts --type js packages/nextjs/src/app-router/server/__tests__/ || echo "No tests directory found"
# Search for machine token or API key auth tests
rg -A 10 -B 5 "machine_token|api_key|MachineAuthObject" --type ts --type js packages/nextjs/src/app-router/server/__tests__/ || echo "No tests directory found"Length of output: 76688
Conditional redirect methods for session tokens – approve with security review and add missing tests
The change correctly restricts redirectToSignIn/redirectToSignUp to SessionToken cases, aligning behavior with token semantics.
However, no unit tests currently cover this branch in packages/nextjs/src/app-router/server/auth.ts.
• Tag clerk/security for review of the updated authentication logic.
• Add a new test suite under packages/nextjs/src/app-router/server/__tests__/auth.test.ts covering:
- When
authObject.tokenType === TokenType.SessionToken, the returned object includes workingredirectToSignInandredirectToSignUpmethods. - When
authObject.tokenType !== TokenType.SessionToken, the returned object does not expose redirect methods (calls should error or be undefined).
🤖 Prompt for AI Agents
In packages/nextjs/src/app-router/server/auth.ts lines 182-186, the code
conditionally adds redirectToSignIn and redirectToSignUp methods only for
SessionToken token types. To fix, request a security review from the
clerk/security team for this authentication logic update. Then, add a new test
suite in packages/nextjs/src/app-router/server/__tests__/auth.test.ts that
verifies when tokenType is SessionToken, the returned object includes
functioning redirect methods, and when tokenType is not SessionToken, these
redirect methods are absent or cause errors.
There was a problem hiding this comment.
@wobsoriano Do types already cover this case ?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
@panteliselef yes, but want to make sure machine auth objects dont include redirect functions at runtime as well
packages/nextjs/src/server/__tests__/getAuthDataFromRequest.test.ts
Outdated
Show resolved
Hide resolved
packages/nextjs/src/server/__tests__/getAuthDataFromRequest.test.ts
Outdated
Show resolved
Hide resolved
| if (authObject.tokenType === TokenType.SessionToken) { | ||
| return Object.assign(authObject, { redirectToSignIn, redirectToSignUp }); | ||
| } | ||
|
|
||
| return authObject; |
There was a problem hiding this comment.
@wobsoriano Do types already cover this case ?
| secretKey: opts?.secretKey || decryptedRequestData.secretKey || SECRET_KEY, | ||
| publishableKey: decryptedRequestData.publishableKey || PUBLISHABLE_KEY, |
There was a problem hiding this comment.
Parts of the sync versus the async variant seem to be getting out of sync. Can we remove duplication ?
treatPendingAsSignedOut seems to be missing from the async variant, is that on purpose ?
Actually are authStatus, authMessage, authReason ever used when passed as options ?
There was a problem hiding this comment.
Updated and removed redundant logic
| return getSessionAuthDataFromRequest(req, opts); | ||
| }; | ||
|
|
||
| const handleMachineToken = ( |
There was a problem hiding this comment.
Is middleware always a requirement now, or was that the case since the start ? If middleware is not a requirement, don't we still need to support on async verification ?
There was a problem hiding this comment.
Ah, good question! If I understand your question correctly, say someone excludes routes from middleware via the matcher config in the middleware, those routes won't have the encrypted x-clerk-request-data header available right? Meaning there wouldn't be an encrypted machine auth object, and we'll still need async verification...
Edit: Actually when someone excludes a path from middleware and then tries to call the auth() helper, they'll get the error auth() was called but Clerk can't detect usage of clerkMiddleware() so it looks like we're still in the right path. getAuthDataFromRequest function was always a sync until async verification was added.
wobsoriano
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/nextjs/src/server/data/getAuthDataFromRequest.ts (1)
111-143: Address the past review comment about async support.Based on the past review comments, there's a question about whether middleware is always required and if async verification is still needed when routes are excluded from middleware. The current implementation assumes encrypted request data is always available.
This addresses the concern raised in the past review about middleware requirements and async verification support.
🧹 Nitpick comments (1)
packages/nextjs/src/server/data/getAuthDataFromRequest.ts (1)
115-117: Potential performance concern with repeated decryption.The
decryptClerkRequestDatais called in bothcreateAuthOptions(line 53) andgetAuthDataFromRequest(line 116), which could lead to redundant decryption operations.Consider caching the decrypted data to avoid duplicate decryption:
export const getAuthDataFromRequest = (req: RequestLike, opts: GetAuthDataFromRequestOptions = {}): AuthObject => { const { authStatus, authMessage, authReason } = getAuthHeaders(req); opts.logger?.debug('headers', { authStatus, authMessage, authReason }); + const encryptedRequestData = getHeader(req, constants.Headers.ClerkRequestData); + const decryptedRequestData = decryptClerkRequestData(encryptedRequestData); + - const encryptedRequestData = getHeader(req, constants.Headers.ClerkRequestData); - const decryptedRequestData = decryptClerkRequestData(encryptedRequestData); const bearerToken = getHeader(req, constants.Headers.Authorization)?.replace('Bearer ', ''); const acceptsToken = opts.acceptsToken || TokenType.SessionToken; - const options = createAuthOptions(req, opts); + const options = createAuthOptionsWithDecryptedData(req, opts, decryptedRequestData);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/nextjs/src/server/__tests__/getAuthDataFromRequest.test.ts(3 hunks)packages/nextjs/src/server/data/getAuthDataFromRequest.ts(4 hunks)packages/nextjs/src/server/utils.ts(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/nextjs/src/server/tests/getAuthDataFromRequest.test.ts
- packages/nextjs/src/server/utils.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/nextjs/src/server/data/getAuthDataFromRequest.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/nextjs/src/server/data/getAuthDataFromRequest.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/nextjs/src/server/data/getAuthDataFromRequest.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/nextjs/src/server/data/getAuthDataFromRequest.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/nextjs/src/server/data/getAuthDataFromRequest.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/nextjs/src/server/data/getAuthDataFromRequest.ts
**/*
⚙️ CodeRabbit Configuration File
**/*: If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.Whenever reviewing a pull request, if there are any changes that could impact security, always tag
clerk/securityin the PR.Security-impacting changes include, but are not limited to:
- Changes to authentication logic or mechanisms (e.g. login, session handling, token issuance)
- Any modification to access control, authorization checks, or role-based permissions
- Introduction or modification of hashing algorithms, signature verification, or cryptographic primitives
- Handling of sensitive data (e.g. passwords, tokens, secrets, PII)
- Integration with external identity providers (e.g. SSO, OAuth, OpenID Connect)
- Modifications to security headers, cookie flags, CORS policies, or CSRF protections
- Bypass mechanisms (e.g. feature flags, testing overrides) that could weaken protections
- Changes to rate limiting, abuse prevention, or input validation
If you're unsure whether a change is security-relevant, err on the side of caution and tag
clerk/security.Any time that you tag
clerk/security, please do so explicitly in a code comment, rather than within a collapsed section in a coderabbit comment, such as the "recent review details" section. If you do use the team name in any thinking or non-direct-code-comment content, it can be referred to as "clerk security team" to avoid accidentally printing the tag which sends a notification to the team.
Files:
packages/nextjs/src/server/data/getAuthDataFromRequest.ts
🧬 Code Graph Analysis (1)
packages/nextjs/src/server/data/getAuthDataFromRequest.ts (8)
packages/nextjs/src/server/types.ts (1)
RequestLike(9-9)packages/nextjs/src/server/headers-utils.ts (2)
getAuthKeyFromRequest(11-16)getHeader(18-27)packages/backend/src/internal.ts (8)
constants(1-1)SignedInAuthObject(23-23)SignedOutAuthObject(24-24)TokenType(18-18)AuthenticateRequestOptions(11-11)MachineTokenType(19-19)isMachineTokenByPrefix(57-57)getAuthObjectForAcceptedToken(36-36)packages/nextjs/src/server/utils.ts (1)
decryptClerkRequestData(225-254)packages/nextjs/src/server/constants.ts (4)
SECRET_KEY(7-7)PUBLISHABLE_KEY(8-8)API_URL(10-10)API_VERSION(6-6)packages/backend/src/tokens/authObjects.ts (4)
SignedInAuthObject(44-62)SignedOutAuthObject(67-83)AuthObject(162-167)getAuthObjectForAcceptedToken(465-495)packages/backend/src/tokens/types.ts (2)
AuthenticateRequestOptions(19-70)MachineAuthObject(181-183)packages/backend/src/tokens/machine.ts (1)
isMachineTokenByPrefix(21-23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
packages/nextjs/src/server/data/getAuthDataFromRequest.ts (6)
1-7: LGTM: Clean import organization and proper type imports.The imports are well-organized with proper separation of type imports and the addition of
MachineAuthObjecttype import aligns with the new machine token handling functionality.
38-46: Good extraction of auth header logic into a helper function.This refactoring improves code reusability and makes the auth header extraction more explicit and testable.
71-101: Approve the session-specific authentication function.The function correctly handles session token authentication with proper validation and error handling. The explicit comment about machine tokens not being supported provides good clarity.
51-65: Ignore incorrect assertion about removed auth fieldsThe snippet in
packages/nextjs/src/server/data/getAuthDataFromRequest.ts(lines 51–65) already returnsauthStatus,authMessage, andauthReasonas shown. There is no removal of these properties and no further action is needed.Likely an incorrect or invalid review comment.
145-173: handleMachineToken implementation is correct as-isThe existing type assertion is required to satisfy TypeScript’s union-based return type (AuthObject vs. MachineAuthObject). At runtime we know
getAuthObjectForAcceptedTokenreturns a valid machine auth object in this branch, so theas MachineAuthObject<MachineTokenType>cast is safe. TheacceptsOnlySessionTokencheck correctly guards against session-only scenarios, and prefix-based token detection along with pre-validatedrawAuthObjectensures proper validation. No changes are necessary here.
111-143: Authentication flow verified as secure
decryptClerkRequestDatareturns an empty object only when the header is absent and will throw on malformed or invalid payloads- Machine tokens are intentionally handled first via
handleMachineToken, preserving the designed hierarchy- Bearer-token extraction (
getHeader(...)?.replace('Bearer ', '')) and the subsequent validation branches are correctNo further fixes or security reviews are required.
| } from '@clerk/backend/internal'; | ||
| import { decodeJwt } from '@clerk/backend/jwt'; | ||
| import type { PendingSessionOptions } from '@clerk/types'; | ||
| import type { AuthenticateContext } from 'node_modules/@clerk/backend/dist/tokens/authenticateContext'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use relative import path instead of node_modules reference.
The import is referencing the node_modules directory directly, which is brittle and non-standard.
-import type { AuthenticateContext } from 'node_modules/@clerk/backend/dist/tokens/authenticateContext';
+import type { AuthenticateContext } from '@clerk/backend/internal';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import type { AuthenticateContext } from 'node_modules/@clerk/backend/dist/tokens/authenticateContext'; | |
| // packages/nextjs/src/server/data/getAuthDataFromRequest.ts | |
| -import type { AuthenticateContext } from 'node_modules/@clerk/backend/dist/tokens/authenticateContext'; | |
| +import type { AuthenticateContext } from '@clerk/backend/internal'; |
🤖 Prompt for AI Agents
In packages/nextjs/src/server/data/getAuthDataFromRequest.ts at line 21, the
import statement uses a direct node_modules path which is non-standard and
brittle. Change the import to use a relative or package-based path by removing
'node_modules/' and importing directly from
'@clerk/backend/dist/tokens/authenticateContext' to follow standard module
resolution practices.
Description
The
clerkMiddleware()runsauthenticateRequest({ acceptsToken: 'any' })under the hood so it already verifies all type of tokens, but thenauth()was re-verifying machine tokens, causing duplicate API calls.The PR eliminates the duplicate call by reusing the encrypted auth object from middleware.
Resolves USER-2379
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
Summary by CodeRabbit